Skip to content

Conversation

@mrvanes
Copy link
Collaborator

@mrvanes mrvanes commented Oct 21, 2025

To completely separate the issuer from authorization server the issuer_state should not be checked in the authorization endpoint, but simply be accepted as an opaque identifier and afterwards be reflected in the access_token.

The issuer then should compare the previously offered issuer_state against the issuer_state in the access_token in order to verify that the state in the access_token reflects the received claims of the userinfo endpoint.

This PR does not implement the native issuer access_token issuer_state inspection!

@tvdijen tvdijen requested a review from cicnavi October 21, 2025 10:53
@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 8.69565% with 63 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.07%. Comparing base (092bf54) to head (ce15fae).
⚠️ Report is 1 commits behind head on wip-vci.

Files with missing lines Patch % Lines
...edentials/CredentialIssuerCredentialController.php 0.00% 23 Missing ⚠️
src/Services/DatabaseMigration.php 0.00% 18 Missing ⚠️
src/Server/Grants/AuthCodeGrant.php 0.00% 12 Missing ⚠️
src/Entities/AccessTokenEntity.php 40.00% 3 Missing ⚠️
src/Factories/Entities/AuthCodeEntityFactory.php 0.00% 3 Missing ⚠️
src/Entities/AuthCodeEntity.php 33.33% 2 Missing ⚠️
...rc/Factories/Entities/AccessTokenEntityFactory.php 0.00% 1 Missing ⚠️
src/Factories/RequestRulesManagerFactory.php 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             wip-vci     #317      +/-   ##
=============================================
- Coverage      42.31%   42.07%   -0.24%     
- Complexity      1870     1883      +13     
=============================================
  Files            160      160              
  Lines           8565     8620      +55     
=============================================
+ Hits            3624     3627       +3     
- Misses          4941     4993      +52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cicnavi
Copy link
Collaborator

cicnavi commented Oct 22, 2025

Conformance tests are passing, which is good! Tomorrow I'll try to implement issuer state check for the internal issuer, as otherwise I'll miss that check.

@cicnavi
Copy link
Collaborator

cicnavi commented Oct 23, 2025

@mrvanes please check if everything works for you after all the changes.

@mrvanes
Copy link
Collaborator Author

mrvanes commented Oct 24, 2025

Everything still works like a charm after the refinement commits. Thank you!

@cicnavi
Copy link
Collaborator

cicnavi commented Oct 24, 2025

Thanks for the contribution!

@cicnavi cicnavi merged commit 07e7977 into simplesamlphp:wip-vci Oct 24, 2025
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants